ci(release): support release/* branches with version-comparison gating#3530
ci(release): support release/* branches with version-comparison gating#3530
Conversation
|
WalkthroughWorkflows and a PR enhancer were updated to support release/** branches and to propagate a computed is_latest flag through the release pipeline. changesets-pr now derives SOURCE_BRANCH from the PR ref, fetches the corresponding changeset-release branch, reads the package version from that branch, and passes SOURCE_BRANCH to the enhancer. release.yml compares the new version to npm dist-tags.latest, emits is_latest and dist_tag outputs, and forwards them to downstream steps. publish workflows accept is_latest and perform semver-aware Docker tagging (adding release-MAJOR.MINOR and optionally v4-beta). scripts/enhance-release-pr.mjs obtains release context and renders a “Release prep” section in PR bodies. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Detailed summary
No public API or exported symbol declarations were changed. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Lets us ship a patch (e.g. 4.4.6) from a release/4.4.x branch without
including unreleased work merged into main, and without the patch
clobbering floating tags incorrectly.
The release-pipeline pieces this touches and how each behaves now:
npm dist-tag latest if version > current latest, else release-<M.m>
Docker :v4-beta same gate (highest version only)
Docker :release-X.Y new per-line floating tag, always set on a semver build
GitHub release --latest=true|false set explicitly (no auto-detect)
How the gate is computed:
release.yml's 'Compare new version to current latest' step queries
npm view @trigger.dev/sdk dist-tags.latest, compares via sort -V,
sets is_latest=true|false. Drives every floating tag.
Triggers / refs:
- pull_request:branches[main, release/**]
- if-conditions allow head.ref starting with 'changeset-release/'
- workflow_dispatch ref must be reachable from main OR a release/* branch
- changesets-pr.yml fires on push to release/** too; PR-enhance step
discovers source branch dynamically (no more hardcoded changeset-release/main)
Other changes:
- gh release create: drop --target main (tag carries right commit)
- dispatch-changelog payload includes is_latest so the marketing site
can render lagged-line releases differently
- enhance-release-pr.mjs prepends a Release prep header on release/*
branches showing version, current latest, and whether the PR will
take the latest dist-tag
release-helm.yml unchanged — already creates as draft+prerelease so it
can't claim Latest. publish-worker.yml (coordinator/provider) unchanged
since those don't have a :v4-beta-equivalent floating tag.
Validated end-to-end in ericallam/pkgring-sandbox across both scenarios:
Scenario A (lagged hotfix): latest stays put, only release-X.Y moves
Scenario B (main has unreleased work, hotfix is highest): latest moves
aa4a9a8 to
4268187
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/enhance-release-pr.mjs (2)
436-438: ⚡ Quick winNaive
cmpmay disagree with the workflow'ssort -Vdecision.This per-component numeric comparison breaks on prerelease versions (e.g.,
Number("6-beta")isNaN, and0 || NaNpropagatesNaNthrough the reduce, leavingwillBeLatestfalsy regardless of order). It also under-compares whenversionhas fewer components thancurrentLatest. Meanwhile,release.ymldecides the actual outcome viasort -V, so the PR body header could show the opposite of what gets shipped.Since this only affects display, it's low-risk, but unifying with semver-aware comparison avoids surprising drift between header and reality.
♻️ Replace with shell-out to `sort -V` (matches release.yml exactly) or use the `semver` package
- const cmp = (a, b) => - a.split(".").map(Number).reduce((acc, n, i) => acc || n - (b.split(".").map(Number)[i] ?? 0), 0); - const willBeLatest = cmp(version, currentLatest) > 0; + // Match release.yml's `sort -V` precisely so the PR body header never + // disagrees with the actual is_latest decision made at publish time. + const higher = await new Promise((resolve) => { + execFile( + "sh", + ["-c", `printf '%s\\n%s\\n' "$0" "$1" | sort -V | tail -1`, version, currentLatest], + { maxBuffer: 1024 * 1024 }, + (err, stdout) => resolve(err ? "" : stdout.trim()) + ); + }); + const willBeLatest = higher === version && version !== currentLatest;Confirm whether
semveris already a workspace dependency available to this script — if so,semver.gt(version, currentLatest)is even simpler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/enhance-release-pr.mjs` around lines 436 - 438, The cmp function used to set willBeLatest is naive and fails on prerelease and differing-component versions; replace its logic by using a proper semver comparison (e.g., use semver.gt(version, currentLatest) if the semver package is available in the workspace) or, to exactly match release.yml, shell out to sort -V and compare ordering; update the assignment to willBeLatest to call the chosen semver-aware comparator (referencing cmp, willBeLatest, version, and currentLatest) and remove or deprecate the old cmp implementation.
422-434: 💤 Low value
try/catchis unreachable — the inner Promise never rejects.The Promise constructor only calls
resolve(in botherrand success branches), so theawaitcannot throw and the outercatchblock is dead. Either resolve to a sentinel and check it, or reject inside the callback so the catch becomes meaningful.♻️ Drop the dead try/catch
let currentLatest = "0.0.0"; - try { - const out = await new Promise((resolve) => { - execFile( - "npm", - ["view", "@trigger.dev/sdk", "dist-tags.latest"], - { maxBuffer: 1024 * 1024 }, - (err, stdout) => resolve(err ? "" : stdout.trim()) - ); - }); - if (out && out !== "undefined") currentLatest = out; - } catch { - // fall through with default - } + const out = await new Promise((resolve) => { + execFile( + "npm", + ["view", "@trigger.dev/sdk", "dist-tags.latest"], + { maxBuffer: 1024 * 1024 }, + (err, stdout) => resolve(err ? "" : stdout.trim()) + ); + }); + if (out && out !== "undefined") currentLatest = out;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/enhance-release-pr.mjs` around lines 422 - 434, The current try/catch is dead because the Promise created around execFile only ever calls resolve; update the Promise in the block that calls execFile so it accepts (resolve, reject) and calls reject(err) when the execFile callback receives an error (and resolve with stdout.trim() on success), which makes the outer try/catch meaningful and preserves the existing check that assigns to currentLatest from out; reference symbols: the Promise around execFile, the execFile callback, the out variable, and currentLatest.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 146-149: The script currently silences npm failures and falls back
to CURRENT="0.0.0", which incorrectly treats transient registry errors as "no
latest"; change the logic around the npm view call so you capture its exit
status and output (the CURRENT variable) separately, retry or fail the job if
the npm view command exits non-zero (network/auth/5xx), and only set the "0.0.0"
default when the command succeeded but returned empty/undefined (true "no
latest" case); update the block that sets CURRENT (the npm view invocation and
the CURRENT="0.0.0" fallback) and ensure downstream logic that computes
IS_LATEST uses the distinct failure vs no-latest signal rather than treating
errors as 0.0.0.
---
Nitpick comments:
In `@scripts/enhance-release-pr.mjs`:
- Around line 436-438: The cmp function used to set willBeLatest is naive and
fails on prerelease and differing-component versions; replace its logic by using
a proper semver comparison (e.g., use semver.gt(version, currentLatest) if the
semver package is available in the workspace) or, to exactly match release.yml,
shell out to sort -V and compare ordering; update the assignment to willBeLatest
to call the chosen semver-aware comparator (referencing cmp, willBeLatest,
version, and currentLatest) and remove or deprecate the old cmp implementation.
- Around line 422-434: The current try/catch is dead because the Promise created
around execFile only ever calls resolve; update the Promise in the block that
calls execFile so it accepts (resolve, reject) and calls reject(err) when the
execFile callback receives an error (and resolve with stdout.trim() on success),
which makes the outer try/catch meaningful and preserves the existing check that
assigns to currentLatest from out; reference symbols: the Promise around
execFile, the execFile callback, the out variable, and currentLatest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 811f672f-8055-4742-b4dd-2f2ea7bf83d6
📒 Files selected for processing (6)
.github/workflows/changesets-pr.yml.github/workflows/publish-webapp.yml.github/workflows/publish-worker-v4.yml.github/workflows/publish.yml.github/workflows/release.ymlscripts/enhance-release-pr.mjs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: typecheck / typecheck
🔇 Additional comments (8)
.github/workflows/publish-webapp.yml (1)
17-91: LGTM — semver-aware tagging logic is sound.The
${TAG#v}parameter expansion safely handles both prefixed (v4.4.6) and bare (4.4.6) tags, the per-linerelease-MAJOR.MINORtag is set unconditionally for semver builds, and:v4-betais correctly gated on the workflow-levelis_latestdecision. Boolean→string coercion when passing throughINPUTS_IS_LATESTenv aligns with the bash== truecomparison..github/workflows/publish.yml (1)
11-105: LGTM —is_latestplumbed cleanly to the v4 reusable workflows.
publish-worker(legacy v3) is intentionally not forwarded, which is consistent with:v4-betabeing a v4-only tag..github/workflows/publish-worker-v4.yml (1)
76-97: LGTM — tag generation matchespublish-webapp.yml.Mirrors the webapp publish step's tagging logic, keeping per-line
release-X.Yalways-on for semver and:v4-betagated onis_latest. Duplication with the webapp workflow is acceptable for now given the small size of the block..github/workflows/release.yml (4)
179-179: LGTM — conditional--tagselection is correct.The GHA ternary (
cond && truthy || falsy) cleanly switches betweenpnpm exec changeset publish --tag release-M.mand the default-tag form. Therelease-prefix avoids npm's rejection of dist-tags that parse as a semver range.
321-325: LGTM —is_latestis rendered as a raw JSON boolean.Quoting
versionwhile leavingis_latestunquoted produces valid JSON because the upstream output is always the literal stringtrueorfalse(thecomparestep has noif:guard and always writes the output before any downstream job consumes it). Defensive note: if a future change ever guards thecomparestep on a condition, the output could become empty and break the JSON payload here.
208-211: 💤 Low valueThe
--latest=<bool>syntax is valid and documented.GitHub CLI's
gh release createofficially supports--latest=falseand--latest=truefor explicit control (documented at https://cli.github.com/manual/gh_release_create). The code correctly uses this syntax. Note: There are known behavioral edge cases where--latest=falsemay be ignored in specific scenarios (e.g., when uploading assets), though this doesn't affect the flag's basic syntax support.
76-94: 💤 Low valueThe ref parameter doesn't restrict remote branches fetched by actions/checkout@v6.
With
fetch-depth: 0, all remote branches (includingorigin/release/*) are fetched and available to git operations regardless of therefcheckout parameter. Therefparameter only controls which commit is checked out locally, not what remote refs are accessible for ancestry checks. Nosparse-checkout,filter, or other restrictions are applied in the workflow or other workflows.The suggestion to add an early
if [ -z "$(...)" ]check remains valid for clarity—it would provide a clearer failure message if no release branches exist yet, though the current loop behavior (simply not iterating) is functionally correct.> Likely an incorrect or invalid review comment..github/workflows/changesets-pr.yml (1)
60-74: The code correctly leverageschangesets/[email protected]'s behavior of preserving nested slashes in branch names, creatingchangeset-release/release/X.Y.xverbatim for arelease/X.Y.xbase branch. Git refs and GitHub API handle these slashes without issue.
| CURRENT=$(npm view @trigger.dev/sdk dist-tags.latest 2>/dev/null || true) | ||
| if [ -z "$CURRENT" ] || [ "$CURRENT" = "undefined" ]; then | ||
| CURRENT="0.0.0" | ||
| fi |
There was a problem hiding this comment.
Major: silent npm-view failure can flip a lagged hotfix into :latest.
npm view ... 2>/dev/null || true collapses transient registry failures (network, 5xx, auth) into the same state as "no latest tag yet". Combined with the 0.0.0 default, any hotfix from release/X.Y.x becomes "newer than current latest" and IS_LATEST=true — which then promotes the hotfix to npm latest, Docker :v4-beta, and the GitHub "Latest" badge. That's the exact failure mode this whole PR is designed to prevent.
The 0.0.0 default is only safe when there genuinely is no latest (first publish). After the first release, an empty/error response should fail fast or retry rather than fall through to "becomes latest".
🛡️ Distinguish "no latest yet" from "npm unreachable"
- name: Compare new version to current latest
id: compare
run: |
set -euo pipefail
NEW=$(jq -r '.version' packages/cli-v3/package.json)
- CURRENT=$(npm view `@trigger.dev/sdk` dist-tags.latest 2>/dev/null || true)
- if [ -z "$CURRENT" ] || [ "$CURRENT" = "undefined" ]; then
- CURRENT="0.0.0"
- fi
+ # Retry npm view to ride out transient registry blips. Only fall back
+ # to 0.0.0 on an explicit "no latest yet" response, never on errors —
+ # a silent network failure here would incorrectly promote a lagged
+ # hotfix to :latest / :v4-beta / GitHub "Latest".
+ for i in 1 2 3; do
+ if CURRENT=$(npm view `@trigger.dev/sdk` dist-tags.latest 2>&1); then
+ break
+ fi
+ echo "npm view failed (attempt $i): $CURRENT" >&2
+ CURRENT=""
+ sleep $((i * 5))
+ done
+ if [ -z "$CURRENT" ]; then
+ echo "Error: could not read `@trigger.dev/sdk` dist-tags.latest from npm." >&2
+ exit 1
+ fi
+ if [ "$CURRENT" = "undefined" ]; then
+ # Genuine "no latest dist-tag yet" — first publish.
+ CURRENT="0.0.0"
+ fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 146 - 149, The script currently
silences npm failures and falls back to CURRENT="0.0.0", which incorrectly
treats transient registry errors as "no latest"; change the logic around the npm
view call so you capture its exit status and output (the CURRENT variable)
separately, retry or fail the job if the npm view command exits non-zero
(network/auth/5xx), and only set the "0.0.0" default when the command succeeded
but returned empty/undefined (true "no latest" case); update the block that sets
CURRENT (the npm view invocation and the CURRENT="0.0.0" fallback) and ensure
downstream logic that computes IS_LATEST uses the distinct failure vs no-latest
signal rather than treating errors as 0.0.0.
Adds support for shipping a patch release from a
release/<major>.<minor>.xbranch alongside the existing main-only flow. Use this when main has unreleased changes you can't include in the next release — a customer hotfix on the current line, an emergency patch, etc.When to use this
You're on
4.4.5. Main has merged work queued for4.5.0(a feature, a refactor — anything not ready to ship). A customer hits a bug. Normally you'd be stuck: shipping from main bundles the unreleased work; reverting on main is messy.With this PR you can cut
release/4.4.x, ship4.4.6, and main stays untouched. The patch goes through the normal release pipeline — npm publish, Docker image, Helm chart, GitHub release, marketing-site changelog — and customers runningnpm install @trigger.dev/sdkget it.If you don't need this, don't use it. Normal main releases work exactly as before.
How to ship a patch from a release branch
The flow has five logical steps. Two of them are automated by the existing release pipeline and you don't do anything for them — they're called out here so you understand what's happening.
1. Create the release branch from the last release tag
A release branch is a parallel timeline that starts from a known-released commit and accumulates patches independently of main.
You always cut from a tag (
v4.4.5), not from main HEAD. The tag is immutable and points at exactly the code customers are running. Cutting from main HEAD would pull in any commits or pending changesets that have landed since the tag — defeating the purpose.The branch name follows the pattern
release/<major>.<minor>.x— one branch per minor line.release/4.4.xaccumulates4.4.6,4.4.7, etc. — never a4.5.xrelease.2. Apply the fix
The release branch represents
v4.4.5 + the fix, and nothing else from main. Two ways to get the fix onto the branch:git cherry-pick <sha>for a single commit,git cherry-pick A..Bfor a range.Either way, the result is the same: this branch's tip is
v4.4.5 + the fix.3. Add a changeset (and optionally a
.server-changes/entry)Changesets is the system that tells the release pipeline what to bump. Without a changeset, the pipeline has no work to do — there's no version to publish. Each changeset is a markdown file in
.changeset/declaring which packages bump and by how much.For a patch release, you typically want
patchon the package(s) you fixed. The changesetsfixedconfig will cascade the bump across the linked packages automatically, so you don't need to list every package.If the fix is server-only (webapp behavior, not a published package), use
.server-changes/instead. Both formats are picked up by the pipeline; only.changeset/ones bump versions.4. (Automated) Version PR opens
Pushing to a
release/*branch triggerschangesets-pr.yml, the same workflow that opens release PRs from main. It runschangeset version, applies the bumps, stampsChart.yaml, consumes any.server-changes/, and opens a pull request titledchore: release v4.4.6(the title comes from the post-process script — that's also where the version is computed and shown).The PR's body now starts with a "Release prep" header containing:
lateston npmlatestfloating tags ("yes" if the proposed version is higher than current latest; "no" otherwise — see "What gets published" below for what that means)That header is the at-a-glance check that the release pipeline understood the situation correctly. Review the rest of the PR like any other release PR.
5. Merge the version PR → release ships
Merging triggers
release.yml. From the release pipeline's point of view, a release-branch merge looks identical to a main merge — same publish steps, same Docker tags, same Helm chart push, same marketing-site dispatch. The only thing that changes is which floating tags the publish takes (see below).6. Port the fix back to main
After the release ships, the fix exists on the release branch but not on main. If you don't bring it back, main's next release won't include the fix and the same bug will reappear.
You want to cherry-pick the fix commit only — not the version-bump commit. The version-bump commit is the changeset PR's auto-generated bump (it edits
package.json,Chart.yaml, etc.) and cherry-picking it onto main would conflict with main's existing versions and delete the consumed changeset.Main's next release naturally rolls up the fix at whatever version main is on.
What gets published
A successful release-branch run publishes:
@trigger.dev/{sdk,core,cli-v3,...}@4.4.6ghcr.io/triggerdotdev/trigger.dev:v4.4.6andghcr.io/triggerdotdev/trigger.dev:release-4.44.4.6v4.4.6Whether it also takes the floating tags (
npm latest, Docker:v4-beta, GitHub "Latest" badge, headline placement on the changelog) depends on whether4.4.6is higher than the currentlateston npm:4.4.5(queued work unreleased) →4.4.6 > 4.4.54.4.64.6.0→4.4.6 < 4.6.0(lagged hotfix)4.6.0In the lagged case,
4.4.6still publishes — but customers runningnpm install(no version pin) keep getting4.6.0. Customers who want the patch on the4.4.xline install withnpm install @trigger.dev/[email protected](or pin Docker to:release-4.4).The version PR's "Release prep" header shows which case you're in before you merge.
Common pitfalls
package.jsonconflicts and deletes the consumed changeset on main.release/4.4.xandrelease/4.5.xwork concurrently. The repo-wideconcurrency.grouponrelease.ymlserializes them so they don't collide on tags.release/4.0.xcut six months ago has the.github/workflows/from then. If you need a hotfix on a very old line, expect to update the workflows on that branch first (cherry-pick relevant CI changes from main).Files changed
CI / scripts only. No package or app code changes.
release.yml— version-comparison gating,release/**triggers, propagatesis_latestpublish.yml,publish-webapp.yml,publish-worker-v4.yml—is_latestinput, gate:v4-beta, add:release-<M.m>changesets-pr.yml— trigger onrelease/**, dynamic source-branch handlingscripts/enhance-release-pr.mjs— release-branch context header on the version PRValidation
End-to-end tested in a sandbox repo with the same pipeline shape (3 npm packages, Docker image, Helm chart, cross-repo dispatch). Real npm publishes, real GHCR builds. Confirmed:
is_latestvalue